TRUNK-6605: Implement globalPropertyDeleted in AdministrationServiceImpl#6006
Conversation
Implements globalPropertyDeleted() to reset presentationLocales when LOCALE_ALLOWED_LIST property is deleted, consistent with globalPropertyChanged() behavior.
|
Good implementation! SonarCloud flagged 2 new issues on this PR, worth looking into before merge to keep the quality gate fully clean. Also, consider adding a test for when the deleted property key is NOT |
| // TODO Auto-generated method stub | ||
|
|
There was a problem hiding this comment.
Kindly why did you remove line?
There was a problem hiding this comment.
Those lines were auto-generated placeholder stubs with no actual implementation. Since I've now provided the real implementation for globalPropertyDeleted, the TODO stub was no longer needed and I removed it to keep the code clean.
jwnasambu
left a comment
There was a problem hiding this comment.
Kindly is your ticket Id supposed to read something like this TRUNK-6605: Implement globalPropertyDeleted in AdministrationServiceImpl and not TRUNK-6605: Implement globalPropertyDeleted to reset presentationLocales
You're right, thank you for pointing that out! The commit message should reflect the broader implementation scope. I'll update it to TRUNK-6605: Implement globalPropertyDeleted in AdministrationServiceImpl. |
Thanks for the feedback! I'll look into the 2 SonarCloud issues and address them before merge. I'll also add a test case to verify that deleting a property key other than LOCALE_ALLOWED_LIST doesn't cause any unintended side effects on other properties. |
jwnasambu
left a comment
There was a problem hiding this comment.
Kindly these changes are unrelated to the ticket’s scope and would be better reviewed and tracked separately.
Thank you for the feedback! You're right, the HibernateFormDAOTest changes were unintentionally included due to a merge |
|



Implements globalPropertyDeleted() to reset presentationLocales when LOCALE_ALLOWED_LIST
property is deleted, consistent with globalPropertyChanged() behavior. Added unit test to verify the behavior.
Issue I worked on
see https://issues.openmrs.org/browse/TRUNK-6605
Checklist: I completed these to help reviewers :)